-
Notifications
You must be signed in to change notification settings - Fork 78
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update to LangChain4j 0.32.0 #721
Conversation
@jmartisk would you take a look at the Redis failures please? Error: Tests run: 8, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 25.07 s <<< FAILURE! -- in io.quarkiverse.langchain4j.redis.deployment.RedisEmbeddingStoreTest
Error: io.quarkiverse.langchain4j.redis.deployment.RedisEmbeddingStoreTest.should_add_embedding_with_segment_with_metadata -- Time elapsed: 0.086 s <<< FAILURE!
java.lang.AssertionError:
Expecting actual:
0.9465928971765
to be close to:
1.0
by less than 1% but difference was 5.34071028235%.
(a difference of exactly 1% being considered valid)
at dev.langchain4j.store.embedding.EmbeddingStoreIT.should_add_embedding_with_segment_with_metadata(EmbeddingStoreIT.java:46)
at java.base/java.lang.reflect.Method.invoke(Method.java:568)
at io.quarkus.test.QuarkusUnitTest.runExtensionMethod(QuarkusUnitTest.java:500)
at io.quarkus.test.QuarkusUnitTest.interceptTestMethod(QuarkusUnitTest.java:414)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1511) |
Yeah, looking. |
Ok so the problem is this new line: which adds a metadata field or type So I think we can, in this case adjust the guessing mechanism to treat the UUID as a String instead of a Number. But, FYI @langchain4j , maybe we should put in place some rules about what values you can put in the metadata. You probably shouldn't be allowed to put any arbitrary objects in there, because it wouldn't be clear how to store them in each underlying embedding store type, and how to perform |
@langchain4j Ah, sorry I just noticed you already have some checks in place, and explicitly allow UUID along with numbers and strings. That's all good then :) |
Ok great, now we have some failures in Milvus instead |
lovely... |
Co-authored-by: Georgios Andrianakis <geoand@gmail.com>
The span now contains attributes that are defined by https://github.com/open-telemetry/semantic-conventions/blob/main/docs/gen-ai/gen-ai-spans.md
The metrics conform to what is defined by https://github.com/open-telemetry/semantic-conventions/blob/main/docs/gen-ai/gen-ai-spans.md Closes: #664
I squashed the the last two (test fix) commits into the first one so we don't have broken intermediate states |
Also includes: